Skip to content

feat(workspace): name tonight's first length change on the map - #1124

Closed
seonghobae wants to merge 8 commits into
developfrom
feat/workspace-first-duration-change
Closed

feat(workspace): name tonight's first length change on the map#1124
seonghobae wants to merge 8 commits into
developfrom
feat/workspace-first-duration-change

Conversation

@seonghobae

@seonghobae seonghobae commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

Product outcome

Name tonight's first consecutive section-length change on the ready rehearsal map and tell the player to count the new length in before that section. When every named section lasts the same number of seconds, keep that length instead of inventing a reset. The matching Section Roadmap card repeats the next count-in.

This is a BandScope-owned buyer gap on shipped section.timeRange evidence. It does not replace #1122 (first groove change), #991 (first groove), #828/#770 (MIR), #811 (CLI authority), or #897 (Storybook tokens).

Exact current identity

  • Protected target: develop@749511c3ad4000090048718f685c6bee6b3d2c25.
  • Exact current head: 3c9eb80f38170ee508347f43052c71f9a8048855.
  • Branch: feat/workspace-first-duration-change.

Current exact scope

  • firstDurationChange validates every meaningful runtime section ID for uniqueness before deriving any duration evidence, so an ineligible or later duplicate cannot create an ambiguous card target or bypass validation through an early return.
  • It then walks labeled sections in form order, skips unlabeled or invalid timeRange evidence, and returns the first consecutive pair whose integer duration differs.
  • Same-length forms keep the first named duration and tell the player to count that length in before the first labeled section.
  • Missing named durations fail closed and ask for an ear check before counting in.
  • Stable section IDs own roadmap targeting; display labels never substitute for card identity. Repeated labels therefore cannot mark multiple cards.
  • Ready workspace still names tonight's first playable range and the next instrument check.
  • Customer-facing EN/KO copy names the next action on both the workspace callout and the arrival roadmap card.

Verification boundary

Desktop unit tests for firstDurationChange, Workspace, and SectionRoadmap passed locally on this exact head (39 tests). Typecheck passed. Fresh repository workflows for exact head 3c9eb80f38170ee508347f43052c71f9a8048855 are non-passing until terminal. Predecessor checks/reviews do not transfer.

Merge gate


Devin Review

Summary by CodeRabbit

  • 새로운 기능

    • 리허설 작업공간에서 곡의 첫 구간 길이 변경 또는 유지 여부를 확인할 수 있습니다.
    • 변경되는 구간의 새 길이와 해당 구간 전에 세어야 할 다음 카운트인을 안내합니다.
    • 영어와 한국어 안내 문구를 추가했습니다.
  • 문서

    • 리허설 준비 및 작업공간 안내 문서에 첫 구간 길이 변경과 다음 카운트인 정보가 반영되었습니다.
  • 테스트

    • 구간 길이 변경, 유지, 누락 및 한국어 안내에 대한 검증을 추가했습니다.

Tell the player to count the new section length in before that arrival,
or keep tonight's held length when every named section lasts the same.
@seonghobae seonghobae added area: ui-ux Frontend, interaction, design, or user experience priority: medium Normal-priority or P2 work type: feature New or expanded product capability scope: product-gap Customer-visible product gap labels Sep 1, 2026 — with Grok (by xAI)

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note

This report is out of date. Scroll down for Devin Review's latest report on this PR.

Devin Review found 4 potential issues.

Devin Review

Comment thread apps/desktop/src/features/workspace/firstDurationChange.ts Outdated
Comment thread apps/desktop/src/features/workspace/firstDurationChange.ts
Comment on lines +72 to +85
const seenSectionIds = new Set<string>();
for (const sectionValue of runtimeSong.sections) {
if (!isRuntimeObject(sectionValue)) {
continue;
}
const sectionId = meaningfulRangeText(sectionValue.id);
if (!sectionId) {
continue;
}
if (seenSectionIds.has(sectionId)) {
return null;
}
seenSectionIds.add(sectionId);
}

@devin-ai-integration devin-ai-integration Bot Sep 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Info: Duplicate IDs suppress guidance

Any repeated meaningful section ID disables both duration surfaces. This strict failure avoids ambiguous roadmap targeting, including when only one duplicate has usable timing.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +32 to +52
export function sectionDurationSeconds(timeRangeValue: unknown): number | null {
if (!isRuntimeObject(timeRangeValue)) {
return null;
}
const start = timeRangeValue.start;
const end = timeRangeValue.end;
if (
typeof start !== "number" ||
typeof end !== "number" ||
!Number.isFinite(start) ||
!Number.isFinite(end) ||
!Number.isInteger(start) ||
!Number.isInteger(end) ||
start < 0 ||
start > MAX_SECTION_TIME_SECONDS ||
end <= start ||
end > MAX_SECTION_TIME_SECONDS
) {
return null;
}
return end - start;

@devin-ai-integration devin-ai-integration Bot Sep 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Info: Counts use rounded boundaries

Displayed lengths follow the shared integer timing contract. Near-boundary acoustic durations can differ by one second after the engine truncates their endpoints.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@coderabbitai

coderabbitai Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

Next included review available in 47 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: ee1c9ea5-1119-44bf-9cc6-03dec784476e

📥 Commits

Reviewing files that changed from the base of the PR and between 71c6cef and 8f46843.

📒 Files selected for processing (3)
  • apps/desktop/src/features/workspace/firstDurationChange.ts
  • apps/desktop/src/lib/analysis.test.ts
  • apps/desktop/src/lib/analysis.ts
📝 Walkthrough

Walkthrough

리허설 곡에서 첫 구간 길이 변경 또는 유지를 계산합니다. 작업공간에는 상태 안내를 표시하고, 로드맵에는 대상 구간의 다음 카운트인 문구를 표시합니다. 시간 증거 검증, 다국어 문자열, 테스트, 문서가 추가되었습니다.

Changes

구간 길이 안내

Layer / File(s) Summary
시간 증거와 길이 계산
apps/desktop/src/lib/rehearsalTimingEvidence.ts, apps/desktop/src/features/workspace/firstDurationChange.*
시간 범위를 검증하고 합성 시간 범위를 식별합니다. 폼 순서에서 첫 번째 길이 변경 또는 동일 길이 상태를 계산합니다. 입력 오류와 중복 구간 ID는 null로 처리합니다.
작업공간 길이 안내
apps/desktop/src/features/workspace/Workspace.*, apps/desktop/src/locales/*/common.json
Workspace에 첫 길이 상태 콜아웃을 추가했습니다. 변경, 유지, 확인 필요 상태에 대한 영어와 한국어 문자열을 추가했습니다.
로드맵 대상 구간 안내
apps/desktop/src/features/workspace/SectionRoadmap.*
길이 변경 대상 구간 카드에 다음 카운트인 문구를 표시합니다. 반복 라벨이 있어도 구간 ID로 대상 카드를 판별합니다.
설계 및 변경 기록 반영
AGENTS.md, ARCHITECTURE.md, CLAUDE.md, CHANGELOG.md
첫 구간 길이 변경과 다음 카운트인 안내를 프로젝트 설명, 아키텍처 문서, 변경 로그에 반영했습니다.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Merge Risk: 🟡 Moderate · up to 71c6c

The new rehearsal guidance can treat parser-generated fallback timing as a real one-second section, giving legacy songs incorrect count-in instructions and reporting unchanged lengths; merge should wait until synthetic timing is rejected.

Sequence Diagram(s)

sequenceDiagram
  participant Workspace
  participant firstDurationChange
  participant TranslationCatalog
  Workspace->>firstDurationChange: song 전달
  firstDurationChange-->>Workspace: 첫 길이 상태 반환
  Workspace->>TranslationCatalog: 상태별 번역 키와 토큰 전달
  TranslationCatalog-->>Workspace: 안내 문자열 반환
  Workspace-->>Workspace: first-duration-change 콜아웃 렌더링
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 제목은 워크스페이스 지도에서 오늘 첫 구간 길이 변경을 표시하는 주요 변경 사항을 정확하고 간결하게 설명합니다.
Docstring Coverage ✅ Passed Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 7 files. (6 skipped: 6 …
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 7 files. (6 skipped: 6 unsupported.)

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/workspace-first-duration-change

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@seonghobae
seonghobae changed the base branch from develop to feat/workspace-first-groove-change September 1, 2026 21:08
@seonghobae
seonghobae changed the base branch from feat/workspace-first-groove-change to develop September 1, 2026 21:08

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note

This report is out of date. Scroll down for Devin Review's latest report on this PR.

Devin Review found 1 new potential issue.

Devin Review

Comment on lines +32 to +52
export function sectionDurationSeconds(timeRangeValue: unknown): number | null {
if (!isRuntimeObject(timeRangeValue)) {
return null;
}
const start = timeRangeValue.start;
const end = timeRangeValue.end;
if (
typeof start !== "number" ||
typeof end !== "number" ||
!Number.isFinite(start) ||
!Number.isFinite(end) ||
!Number.isInteger(start) ||
!Number.isInteger(end) ||
start < 0 ||
start > MAX_SECTION_TIME_SECONDS ||
end <= start ||
end > MAX_SECTION_TIME_SECONDS
) {
return null;
}
return end - start;

@devin-ai-integration devin-ai-integration Bot Sep 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Info: Duration bounds remain consistent

sectionDurationSeconds matches the shared parser’s integer, ordering, and maximum-time rules. Valid parsed songs retain their duration evidence.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Collaborator Author

Succession-in-progress evidence: the exact firstDurationChange.ts + firstDurationChange.test.ts blobs from this head (3c9eb80f38170ee508347f43052c71f9a8048855) are now preserved in canonical overlapping writer #1122 at d40bd4550d728b60368114580dcd5cd73adf7631. No check/review evidence transfers. This PR stays open for now because its Workspace/SectionRoadmap/EN-KO mounted behavior and integration regressions have not yet been reconstructed on #1122. Do not add further same-boundary work here; #1122 owns the consolidation. Close as superseded only after that mounted behavior is demonstrated on the canonical head.

coderabbitai[bot]

This comment was marked as resolved.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 new potential issue.

Devin Review

export async function loadProject(): Promise<RehearsalSong> {
const response = await invokeAnalysis("load_project");
return parseRehearsalSong(response);
return parseRehearsalSongWithTimingEvidence(response);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Saved legacy timings become measured

After a legacy project is saved and reopened, parseRehearsalSongWithTimingEvidence treats its serialized placeholder as explicit timing. The rehearsal map then presents a fabricated section length and count-in.

Prompt for agents
Preserve synthetic section-timing provenance across the save/load round trip. loadProject currently marks migrated ranges with a symbol, but saveProject calls parseRehearsalSong and sends JSON through Tauri; both serialization steps discard symbol keys while retaining the generated one-second timeRange. A later load therefore cannot distinguish that placeholder from measured timing. Update the project persistence boundary or project format so synthetic ranges remain identifiable after saving, while explicit measured one-second ranges remain eligible. Add a regression test covering legacy load, save payload, and reload.
Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Collaborator Author

Superseded by canonical first-change owner #1122 at exact head 4eabf558e60547769266a9322501fbbca93ca7b9 after live semantic reconciliation.

Technical succession evidence:

This closure does not transfer #1124 checks, reviews, or approvals. #1122's new exact head requires its own fresh protected-gate and independent-review evidence.

@seonghobae seonghobae closed this Sep 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: ui-ux Frontend, interaction, design, or user experience priority: medium Normal-priority or P2 work scope: product-gap Customer-visible product gap type: feature New or expanded product capability

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant